The hardcoded IPC path ipc:///tmp/flexkv_test can lead to conflicts if multiple instances of this example are run concurrently on the same machine. This could cause unexpected failures or race conditions. To ensure the example is robust and can be run in parallel, a unique path should be generated for each run, for example by appending the process ID.

The default model path "/data0/models/Qwen3/Qwen3-32B" is hardcoded to a specific developer's environment. This will cause the example to fail for other users who do not have this path. To make the example runnable for everyone, it's better to remove the default value and require the user to set the MODEL_PATH environment variable, raising an error if it's not set.

Using time.sleep(2) to wait for an asynchronous offload task to complete is unreliable. The task might take more or less time depending on system load and other factors. This can lead to flaky behavior in the example, where it might fail intermittently or have unnecessary delays. A more robust synchronization mechanism should be used. If the FlexKV API provides a way to block until the operation is complete (e.g., a future, an event, or a blocking call), it should be used instead. This would make the example more reliable and demonstrate better programming practices.

Absolutely right that time.sleep(2) is not a reliable synchronization mechanism. The offload task here is an asynchronous background operation managed internally by FlexKV. Unfortunately, the current LLM (offline inference) API does not expose a blocking interface to wait for the offload to complete — the synchronization (wait_for_save) happens internally at the worker level during the forward pass. In practice, 2 seconds is more than sufficient for the async KV transfer task to complete in typical scenarios, so this works reliably for the purpose of this example.

Similar to the previous comment, using time.sleep(2) here is unreliable for synchronizing with an asynchronous task. This can make the example flaky. Please use a proper synchronization primitive from the FlexKV API if one is available.

This is the same case as the previous comment — the 2-second sleep is intentional here for the same reason.

Looking at the vLLM APIs used by this connector:

# vllm
from vllm.distributed.kv_transfer.kv_connector.v1.base import (
    KVConnectorMetadata, KVConnectorRole)
from vllm.distributed.kv_transfer.kv_connector.v1.metrics import KVConnectorStats
from vllm.distributed.parallel_state import get_tp_group

if TYPE_CHECKING:
    from vllm.config import VllmConfig
    from vllm.v1.core.sched.output import SchedulerOutput
    from vllm.attention.backends.abstract import AttentionMetadata
    from vllm.distributed.kv_events import KVCacheEvent
    from vllm.forward_context import ForwardContext
    from vllm.v1.core.kv_cache_manager import KVCacheBlocks
    from vllm.v1.request import Request
    from vllm.v1.outputs import KVConnectorOutput

I don't think we (the vLLM project) commit to backwards compatibility with these APIs, so it's quite likely the connector will need to be updated as new vLLM releases come out

(This is really just an FYI - I don't have an alternate recommendation, and I can't even be confidently specific about which of these APIs we do expect to remain backwards compatible)

Thanks for the heads-up! We understand that these internal APIs are subject to change and vLLM does not guarantee backwards compatibility. To address this, we have added type checking and compatibility logic in the FlexKV-vLLM adapter code to better handle potential changes. We (the FlexKV team) will keep an eye on vLLM updates and maintain the connector to ensure compatibility with future releases.

You could also setup a test job in CI that installs FlexKV so we know when it breaks

You could also setup a test job in CI that installs FlexKV so we know when it breaks

Thanks for the suggestion! We've added a CI workflow in the FlexKV repository to track vLLM API compatibility: .github/workflows/vllm-compat-test.yml

It runs on a daily schedule and covers three checks:

All vLLM internal symbols used by FlexKVConnectorV1 are still importable.

FlexKVConnectorV1 implements every abstract method required by KVConnectorBase_V1.

All overridden methods still exist in the base class.

We think it's more appropriate to maintain this CI test in the FlexKV repository itself rather than adding it to vLLM's CI, to avoid introducing external dependencies into vLLM's test pipeline.

Could you consider pinning to a branch or commit? I'm not sure how active development is

Thanks for the suggestion! We intentionally keep this as a clone of the main branch rather than pinning to a specific tag/commit, because FlexKV maintains a dedicated CI workflow (vllm-compat-test.yml) that continuously validates compatibility with vLLM. This ensures the main branch of FlexKV is always compatible with the corresponding vLLM version. We (the FlexKV team) are committed to keeping the main branch in a release-ready state at all times. Pinning to a specific commit would require frequent updates to this example whenever FlexKV releases bug fixes.

You could also setup a test job in CI that installs FlexKV so we know when it breaks

Seems you are missing the MODEL_PATH env set in your example. You could also just make it a CLI arg

Thanks! I've updated the example to use --model as a required CLI argument instead of the MODEL_PATH env var. The usage is now:

python examples/offline_inference/prefix_caching_flexkv.py \
    --model /path/to/your/model

It would be best to share a link or instructions to install so the user can be guided properly

Thanks for the feedback! I've updated the docstring to include both a link and step-by-step installation instructions. The current code now reads:

Installation:
See https://github.com/taco-project/FlexKV for installation instructions.
Quick start::

    git clone git@github.com:taco-project/FlexKV.git
    cd FlexKV && bash build.sh

Additionally, the ImportError message also directs users to the GitHub repo:

 raise ImportError(
      "FlexKV is not installed. Please install it to use "
      "FlexKVConnectorV1. See https://github.com/taco-project/FlexKV "
      "for installation instructions."
 ) from e

This should give users clear guidance

Are you making use of both these paths here?
Usually connectors either load by layer blocking or load all layers async, but I dont see a flag to switch logic here

Thanks for the question, and apologies for the confusing docstrings!

To clarify the design: FlexKV uses a scheduler-side transfer model (similar to NIXL's scheduler-side coordination) — all KV transfers are coordinated by the scheduler connector — build_connector_meta calls launch_tasks() to kick off async transfers, and update_connector_output polls query_finished_task() for completion. KV blocks are moved directly between the FlexKV server and vLLM's GPU memory without any worker-side intervention during the forward pass.

So to answer your question: we are not using both paths — start_load_kv, wait_for_layer_load, save_kv_layer, and wait_for_save are all currently no-ops. We keep them because:

1. KVConnectorBase_V1 requires implementing these methods
2. They serve as extension points for a potential future worker-side layer-pipelining optimisation

We've updated the docstrings in the latest commit to make this explicit. Sorry for the confusion caused by the misleading comments!